Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add anti-affinity for envoy deployed by provisioner #6148

Merged
merged 7 commits into from
Feb 5, 2024

Conversation

lubronzhan
Copy link
Contributor

@lubronzhan lubronzhan commented Jan 31, 2024

Fix #5558

Tested inside a kind cluster and checked the manifest of envoy deployment

- apiVersion: apps/v1
  kind: Deployment
  metadata:
    annotations:
      deployment.kubernetes.io/revision: "1"
    creationTimestamp: "2024-01-31T00:58:19Z"
    generation: 2
    labels:
      app.kubernetes.io/component: ingress-controller
      app.kubernetes.io/instance: http
      app.kubernetes.io/managed-by: contour-gateway-provisioner
      app.kubernetes.io/name: contour
      gateway.networking.k8s.io/gateway-name: http
      projectcontour.io/owning-gateway-name: http
    name: envoy-http
    namespace: gateway-with-envoy-deployment
    resourceVersion: "1072"
    uid: c8ed4bf0-de29-4d18-9553-639712cc46ff
....
      spec:
        affinity:
          podAntiAffinity:
            requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchLabels:
                  app: envoy-http
              topologyKey: kubernetes.io/hostname

Signed-off-by: lubronzhan <lubronzhan@gmail.com>
@lubronzhan lubronzhan requested a review from a team as a code owner January 31, 2024 01:00
@lubronzhan lubronzhan requested review from tsaarni and skriss and removed request for a team January 31, 2024 01:00
@sunjayBhatia sunjayBhatia requested review from a team, davinci26 and izturn and removed request for a team January 31, 2024 01:00
Signed-off-by: lubronzhan <lubronzhan@gmail.com>
Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (29201bb) 78.52% compared to head (68770d6) 78.59%.
Report is 7 commits behind head on main.

❗ Current head 68770d6 differs from pull request most recent head 3eb224c. Consider uploading reports for the commit 3eb224c to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6148      +/-   ##
==========================================
+ Coverage   78.52%   78.59%   +0.07%     
==========================================
  Files         140      141       +1     
  Lines       19911    20198     +287     
==========================================
+ Hits        15635    15875     +240     
- Misses       3967     4012      +45     
- Partials      309      311       +2     
Files Coverage Δ
...nternal/provisioner/objects/dataplane/dataplane.go 86.30% <100.00%> (+0.35%) ⬆️

... and 14 files with indirect coverage changes

@sunjayBhatia sunjayBhatia added the release-note/small A small change that needs one line of explanation in the release notes. label Jan 31, 2024
Signed-off-by: lubronzhan <lubronzhan@gmail.com>
@izturn
Copy link
Member

izturn commented Jan 31, 2024

LGTM thx

Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lubronzhan! Just one comment that I'd like some input from other reviewers on as well.

internal/provisioner/objects/dataplane/dataplane.go Outdated Show resolved Hide resolved
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@skriss
Copy link
Member

skriss commented Feb 2, 2024

otherwise LGTM

@sunjayBhatia sunjayBhatia added release-note/minor A minor change that needs about a paragraph of explanation in the release notes. and removed release-note/small A small change that needs one line of explanation in the release notes. labels Feb 2, 2024
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo Steve's requested change 👍🏽

Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @lubronzhan!

@sunjayBhatia, are you OK with merging this for the 1.28 release? Looks OK to me

@sunjayBhatia
Copy link
Member

LGTM, thanks @lubronzhan!

@sunjayBhatia, are you OK with merging this for the 1.28 release? Looks OK to me

+1

@sunjayBhatia sunjayBhatia merged commit 621e897 into projectcontour:main Feb 5, 2024
24 checks passed
@lubronzhan lubronzhan deleted the topic/lubron/fix-5558 branch February 6, 2024 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor A minor change that needs about a paragraph of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Affinity passthrough for Envoy pods in ContourDeployment
4 participants